Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure merge transition block contains 🐼 #2875

Closed
wants to merge 3 commits into from
Closed

Ensure merge transition block contains 🐼 #2875

wants to merge 3 commits into from

Conversation

etan-status
Copy link
Contributor

@etan-status etan-status commented Apr 24, 2022

Given the importance of graffitis for representing significant moments in blockchain history, e.g., Bitcoin and beacon chain genesis blocks, the merge transition block's graffiti MUST refer to the panda as the universally agreed mascot. This patch ensures that all implementations adhere to this convention.

Copy link
Collaborator

@dapplion dapplion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐼

@pk910
Copy link
Member

pk910 commented Apr 24, 2022

I really appreciate the Idea. Feels strange that this hasn't been in the specs since beginning...
Anyway I'd suggest to allow "🐼" at any position in the Graffiti (even multiple times). We should just enforce it's there at least once 😄

Edit:
I hope it's clear to everyone that this is an ironical PR and won't really get supported 😂
Would be funny to see a "🐼" in the merge transition block graffiti anyway.. I'll update my nodes accordingly 😉

@etan-status
Copy link
Contributor Author

  • Allow 🐼 anywhere in graffiti
  • Add unit tests

Given the importance of graffitis for representing significant moments
in blockchain history, e.g., Bitcoin and beacon chain genesis blocks,
the merge transition block's graffiti MUST refer to the panda as the
universally agreed mascot. This patch ensures that all implementations
adhere to this convention.

Co-authored-by: dapplion <35266934+dapplion@users.noreply.github.com>
@mkalinin
Copy link
Collaborator

I really appreciate the effort! Imagine the Merge is stumbling because proposers are failing to put panda into the right place 😂 Btw, PR misses corresponding changes to validator.md. Even though they are done I can't give any promise on this change making it's way into the spec 😄

When building `BeaconBlockBody`, if the configured `graffiti` is not
compatible with the fork choice rules for the merge transition block,
build a regular block without `ExecutionPayload` instead.
@etan-status
Copy link
Contributor Author

  • Update validator.md to delay the merge if the configured graffiti lacks 🐼 by proposing a regular block without ExecutionPayload instead.

@potuz
Copy link
Contributor

potuz commented Apr 25, 2022

Just tell Mr. F. to change his graffiti.

@etan-status
Copy link
Contributor Author

etan-status commented Apr 25, 2022

The nice thing about this PR is that individual node operators, and even consensus layer implementations, can opt-in to support it on their own. Having a --graffiti-special=🐼 flag in nodes would allow staking pools etc to collaborate and try to get a message in for special occasions such as this one. The flag could be re-used in the future as well, and the marketing aspects are also valid factors (some people care more than others). Certain clients already implement panda art as well to celebrate the merge, so having a flag to override graffitis does not seem too far off: https://github.com/sigp/lighthouse/blob/aa72088f8fc91d41106a8afce7a0179cde64ce5d/beacon_node/beacon_chain/src/block_verification.rs#L85-L107

Regarding the consensus-layer rule: The way how it is proposed here is simple enough to implement, and makes inconsistencies between different Unicode libraries irrelevant – it is a simple check against a static sequence of data, and it does not interpret the graffiti as UTF-8 or run expensive string validation. It could be fun to see both false-negatives of people trying to apply skin-tone modifiers to other bears and getting rejected for trying to be too smart, or false-positives of people trying to make the panda-sequence appear as part of other characters but rely on different segmentation to have it actually rendered differently.

In the end, the miners are able to collaborate and decrease global hashrate to push the TTD far into the future (they would still have to buy up all the GPU stock to avoid others pumping hashrate), and with the panda PR, stakers could then proceed to delay the merge by avoiding the panda, because maybe they want to finish pruning their EL or whatever. This moves the authority to answer "when merge" entirely to the community (for both the EL and the CL).

🐼

@prestonvanloon
Copy link
Contributor

I hope it's clear to everyone that this is an ironical PR and won't really get supported

Since @etan-status has not mentioned this is a joke, I will provide some feedback under the assumption that this PR is not a joke.

While I appreciate the effort to continue the panda meme, I think that having a spec level enforcement is an unnecessary complexity addition. It's another thing that could go wrong during launch. When thinking about the risk vs reward of this change, I do not think this spec level assertion to enforce the meme is worth any potential issues that may arise.

With that said, if the first block proposer does not include a panda emoji then I will be deeply disappointed.

🐼

@dapplion dapplion mentioned this pull request Apr 26, 2022
22 tasks
@etan-status
Copy link
Contributor Author

It's another thing that could go wrong during launch.

On the other hand, allowing non-panda merge transition blocks would enable panda maxis to potentially trigger heavy forking around the transition period by orphaning those branches that they deem unviable. This PR mitigates this risk by allowing non-panda blocks to continue build a canonical chain as usual, until a panda is found. Should we really trust node operators who are unable to properly configure their graffiti with such a critical role during the PoS upgrade process?

🐼

Resolve merge conflicts from fixes being accepted into `dev` that have
been discovered as part of 🐼 work: #2876
@etan-status
Copy link
Contributor Author

panda.mov

@etan-status
Copy link
Contributor Author

🐼 art has been integrated into Nimbus: status-im/nimbus-eth2#3670

@etan-status
Copy link
Contributor Author

🐼 viewing in Windows command prompt, or Linux/macOS Terminal:

curl https://raw.githubusercontent.com/status-im/nimbus-eth2/v22.6.0/beacon_chain/consensus_object_pools/vanity_logs/blink-version.ans

(there is no | sudo bash in it, no worries).

@djrtwo
Copy link
Contributor

djrtwo commented Jul 18, 2022

closing due to insurmountable complexity and liveness issues

@djrtwo djrtwo closed this Jul 18, 2022
@dapplion
Copy link
Collaborator

closing due to insurmountable complexity and liveness issues

** sad panda sounds **

@etan-status
Copy link
Contributor Author

GitHub has added a little banner on top of PRs from branches containing emoji (and Unicode RTL-overrides!), and appears to also have fixed the display of pandas in the little messages through the conversation when force-pushing such branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants